Repository navigation
[16.0][FIX] stock_available_mrp: avoid infinite recursion on sibling-variant kits - #77
alvaro-gmz wants to merge 1 commit into
Conversation
OCA-git-bot
added
series:16.0
mod:stock_available_mrp
…t kits When a phantom BoM uses another variant of its own template as a component, ``explode_bom_quantities`` followed ``first(.bom_ids)`` on that component and looped on a sibling phantom BoM (``bom_ids`` is template-wide via ``_inherits``). After ~130 iterations the accumulated quantity overflowed to ``inf``, produced ``NaN`` and the final ``math.ceil`` raised ``ValueError: cannot convert float NaN to integer``. Use ``mrp.bom._bom_find(..., bom_type='phantom')`` so the lookup honours the variant scope and never surfaces sibling-variant BoMs. Results are cached per call to avoid the SELECT volume warned about in the docstring. A regression test reproduces the trap with a minimal template carrying two variants where one is a phantom kit of the other.
62e9883 to
0feac6f
Compare
StefanRijnhart
left a comment
StefanRijnhart
left a comment
There was a problem hiding this comment.
Someone proposed a bugfix into my v18 migration of this module for a different issue. See the latest commit on #68 (currently 7c1e45c). I ran your tests against that commit, and it seems to fix them. As its solution seems to avoid the BoM type issue. do you think it is a proper fix that can also be applied to 16.0?
| line_quantity = current_qty * current_line.product_qty | ||
|
|
||
| sub_bom = first(current_line.product_id.bom_ids) | ||
| sub_bom = _resolve_bom(current_line.product_id, bom_type="phantom") |
There was a problem hiding this comment.
Is this not a problem if the variant has its own normal type BoM instead of a phantom BoM?
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
Bot
added
the
stale
Problem
When a phantom BoM uses another variant of its own
product.templateas acomponent,
explode_bom_quantitiesenters an infinite recursion thateventually raises
ValueError: cannot convert float NaN to integerfrommath.ceilinsidetools.float_round.Reproducer (covered by the new regression test):
unit variant.
The bug is triggered when reading
immediately_usable_qty(or itsdownstream computed fields such as
immediately_usable_qty_todayonsale.order.line) on the pack variant.Root cause
explode_bom_quantitieswalks the BoM tree by callingfirst(current_line.product_id.bom_ids)on every component to detectnested phantom BoMs.
product.product.bom_idsis template-wide through_inherits, so for the unit variant it returns the phantom BoM of itssibling (the pack) as well. The loop never terminates: each iteration
re-yields the same sibling phantom BoM and multiplies
current_qtybythe line quantity. After ~130 iterations the float overflows to
inf,the next arithmetic step produces
NaNandmath.ceilraises.The same pattern is silently masked when the template has at least one
normalBoM whose id is lower than the phantom one:first()thenreturns the normal BoM and the
if sub_bom.type == "phantom"branch isskipped. The fragility hides behind ordering luck.
Fix
Replace
first(.bom_ids)lookups withmrp.bom._bom_find(..., bom_type="phantom"), which honours the variant scope and does notsurface sibling-variant BoMs. The result is cached per
(product, bom_type)tuple inside the call to keep the SELECT volume bounded — thedocstring already warned about
_bom_findbeing chatty if callednaively per line.
Test
A
TransactionCasereproduces the trap with a minimal templatecarrying two variants where one is a phantom kit of the other. It
asserts three things:
explode_bom_quantities(pack)terminates and yields exactly oneline (
unit × 12).immediately_usable_qtyon the pack variant completeswithout
ValueErrorand yields a real number.explode_bom_quantities(unit)(the component) terminates with anempty result instead of recursing on its sibling's BoM.
Compatibility
Behaviour is unchanged for the legitimate cases where the previous
first(.bom_ids)happened to return the right BoM._bom_findreturnsexactly the BoM that applies to the variant, either variant-specific
(
product_id = variant) or template-level (product_id = False).Sibling-variant BoMs are intentionally excluded — surfacing them on a
variant was the bug.